[codex] namespace sleep under clock - #29907
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b512044faf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| impl ToolExecutor<ToolInvocation> for SleepHandler { | ||
| fn tool_name(&self) -> ToolName { | ||
| ToolName::plain(SLEEP_TOOL_NAME) | ||
| ToolName::namespaced(NAMESPACE, TOOL_NAME) |
There was a problem hiding this comment.
When hooks are enabled and a user already has a PreToolUse/PostToolUse matcher for sleep, this rename changes the hook-facing name from sleep to clocksleep: the default function hook payload uses flat_tool_name(&invocation.tool_name), and the hook engine matches only that canonical name plus explicit aliases. Because SleepHandler does not add a sleep matcher alias, existing hooks that used to audit or block the sleep tool stop running as soon as the model calls the new clock.sleep; please preserve sleep as a matcher alias or override the hook payload while keeping the model-visible namespace.
Useful? React with 👍 / 👎.
Summary
clock.sleepinstead of top-levelsleepclock.curr_timeandclock.sleepin the same model-visible namespace when both features are enabledWhy
Sleep is a clock operation. Grouping it with
clock.curr_timegives the model a more coherent tool surface without changing the sleep feature gate or runtime behavior.Validation
just test -p codex-core sleep_tool_follows_feature_gatejust test -p codex-core any_new_input_interrupts_sleepjust test -p codex-app-server sleep_emits_started_and_completed_items